Кирилл Корняков, Андрей Морозов
Сентябрь 2015
Наши клиенты не смотрят на исходный код.
Почему мы должны держать его в чистоте?
Генеральный директор компании по разработке ПО,
Нижний Новгород, Август 2010
Кто же виноват?
Хорошо |
Плохо |
|
|
Matrix createMatrix(int a1, int a2);
Matrix createMatrix(int width, int height);
List<PECustomerDetailsData> retrieveValidateAndConvertCustomerSpecificDataIntoPresentationEntities();customer.customerName = "Bill"; public void do(); // up
private int i1; // downПрограммист не стал выдумывать и назвал детей
новый сын(1) и новый сын(2)
Что делает следующий фрагмент?
public List<int[]> getThem() {
List<int[]> list1 = new ArrayList<int[]>();
for (int[] x: theList) {
if (x[0] == 4)
list1.add(x);
}
return list1;
}А теперь?
public List<int[]> getFlaggedCells() {
List<int[]> flaggedCells = new ArrayList<int[]>();
for (int[] cell: gameBoard) {
if (cell[STATUS_VALUE] == FLAGGED)
flaggedCells.add(cell);
}
return flaggedCells;
}public List<Cell> getFlaggedCells() {
List<Cell> flaggedCells = new ArrayList<Cell>();
for (Cell cell: gameBoard) {
if (cell.isFlagged()) {
flaggedCells.add(cell);
}
}
return flaggedCells;
}public List<int[]> getThem() {
List<int[]> list1 = new ArrayList<int[]>();
for (int[] x: theList) {
if (x[0] == 4)
list1.add(x);
}
return list1;
}public List<Cell> getFlaggedCells() {
List<Cell> flaggedCells = new ArrayList<Cell>();
for (Cell cell: gameBoard) {
if (cell.isFlagged()) {
flaggedCells.add(cell);
}
}
return flaggedCells;
} int dailyPay = hourlyRate * 8;
double milesWalked = feetWalked / 5280;
int step = width * 4;Не используйте их!
Вместо этого используйте:
WORK_HOURS_PER_DAYFEET_PER_MILEsizeof(int)public class Part {
private String m_dsc;
void setName(String name) {
m_dsc = name;
}
}
public class Part {
private String description;
void setDescription(String description) {
this.description = description;
}
}Имена классов и объектов должны представлять собой
существительные и их комбинации.
Хорошо
CustomerWikiPageAccountПлохо
Manager -> AccountManagerProcessor -> JobProcessorData -> OrderInfo -> HelpMessageИмена методов представляют собой
глаголы или глагольные словосочетания.
String name = employee.getName();
customer.setName("Mike");
if (paycheck.isPosted()) ...Хорошо
public class IncompleteOrder {}
public int currentPosition = -1;
const int WORK_HOURS_PER_DAY = 8;
private bool isBlocked // can, is, has
Плохо
public class incompleteOrder {}
private bool flag;
public const int NUMBEROFCONTEXTS = 10;
private int collectionsize;
private string m_strName;
private byte _array;Какова нормальная длина функции?
In a completely uncommented 2000 line method:
{
{
while (..) {
if (..){
}
... (just putting in the control flow here, imagine another few hundred ifs)
if(..) {
if(..) {
if(..) {
...
(another few hundred brackets)
}
}
} //endif
...endif showed up around line 800! int OverlayFlatVideos(int numberOfFlatVideos,
int currentFrameIdx,
OverlayAllVideosParams^ previewParams,
std::vector<bool>& StreamProcessed,
std::vector<acvCapture*>& flatVideoReaders,
std::vector<double>& fpsFlatVideos,
bool sharedReflection,
std::vector<CvMat*>& reflectionsFlatToDome,
CvSize& fullDomeSize,
std::vector<IplImage*>& masks,
std::vector<IplImage*>& borderSmoothImage,
CvSize& maskSize,
IplImage*& imageFullDome,
CvMat*& tempRef,
double fps,
int& numberOfVideoReaders,
IplImage*& imageReflected,
IplImage*& imageFullDomeCopy,
InterpolationMethod inMethod) // 19 hThreadArray[i] = CreateThread(
NULL, // default security attributes
0, // use default stack size
MyThreadFunction, // thread function name
pDataArray[i], // argument to thread function
0, // use default creation flags
&dwThreadIdArray[i]); // returns the thread identifier
wnd.CreateWnd(hInstance, wcname, NULL, WS_VISIBLE|WS_OVERLAPPEDWINDOW,
NWin::SRect(NWin::SPoint(CW_USEDEFAULT,CW_USEDEFAULT),
NWin::SSize(600,400)), NULL,
LoadMenu(hInstance, resWapp), NULL); static void GetSupportDocFilePath(out string supportDocFilePath) {
supportDocFilePath = new ConfigurationHelper().SupportFilePath;
}
static string GetSupportDocFilePath() {
return new ConfigurationHelper().SupportFilePath;
}float _______ ( float number )
{
long i;
float x2, y;
const float threehalfs = 1.5F;
x2 = number * 0.5F;
y = number;
i = * ( long * ) &y; // evil floating point bit level hacking
i = 0x5f3759df - ( i >> 1 ); // what the fuck?
y = * ( float * ) &i;
y = y * ( threehalfs - ( x2 * y * y ) ); // 1st iteration
// y = y * ( threehalfs - ( x2 * y * y ) ); // 2nd iteration, this can be removed
return y;
} if (splitParameters->projectorVideos == nullptr ||
System::String::IsNullOrEmpty(splitParameters->splitSettings) ||
splitParameters->projectorWidth <= 0 ||
splitParameters->projectorHeight <= 0)
if (timer.HasExpired() && !timer.IsRecurrent())
if (ShouldBeDeleted(timer)) {}
if(isValid == false) {}
if(!canEditPrice) {} public void SendShutDown() {
var handle = GetHandle(device);
if (handle != DeviceHandle.INVALID) {
var err = OpenDevice(handle);
if (err == NULL) {
//
}
else {
Logger.Log("Can't open device. Error: " + err.ToSting());
}
}
else {
Logger.Log("Invalid handle for: " +
device.ToSting());
}
} public void TrySendShutDown() {
try {
SendShutDown();
}
catch() {
// ...
}
}
public void SendShutDown() {
var handle = GetHandle(device);
OpenDevice(handle);
//...
} // When I wrote this, only God and I understood what I was doing
// Now, God only knows
...
// Magic. Do not touch.
...
// sometimes I believe compiler ignores all my comments
...
// I'm sorry.
...
// I am not sure if we need this, but too scared to delete.catch (Exception e) {
//who cares?
}Они полезны?
/*---------------------------------------------------------------
-----------------------
Created by: NANDA
Created Date: 01-AUG-2009
Modified by:
Procedure Description: Fetches menu items based on the given
user permission
Input Parameters: LoginEntry user
Output Parameters: none
----------------------------------------------------------------
--------------------*/
public BEMenuList FetchMenuItems(LoginEntity user) {
...
} ...
// Gets the login user id
// Gets the CRM details
FetchCrmDetails();
... // If the server variable is empty, throw the error message
if (loginUserId == null)
{
throw new Exception("No User Id");
} public void LoadProperties() {
try
{
var propertiesPath = propertiesLocation +
"/" + PROPERTIES_FILE;
var propertiesStream = File.Open(propertiesPath);
loadedProperties.Load(propertiesStream);
}
catch (IOException ex) {
//If file with properties doesn’t exist,
//default settings are loaded
}
}//#region ShowHyperLink
///// <summary>
///// Show the hyperlink controls
///// </summary>
//private void ShowHyperLink()
//{
// asphypCreateAccounts.Visible = true;
// asphypCreateAccounts.NavigateUrl = "CreateAccount.aspx";
// asphypCreateAccounts.Text = WebConstants.CreateAccountHyperLink;
// asphypCreateExtUser.Visible = true;
// asphypCreateExtUser.NavigateUrl = "ManageExternalUsers.aspx";
// asphypCreateExtUser.Text = WebConstants.ExternalUserHyperLink;
//}
//#endregionasplblAcceptDeclineDate.Text = contractHistoryList[0].AcceptedDate.ToString();
if (contractHistoryList[0].IsMultiple)
{
asplblMultiplePublish.Text = "Yes";
}
else
{
asplblMultiplePublish.Text = "No";
}
//if (contractHistoryList[0].IsLegalApprovalRequierd == true)
//{
// asplblLegalApproval.Text = "Yes";
//}
//else
//{
// asplblLegalApproval.Text = "No";
//}
asphdnFileGuid.Value = contractHistoryList[0].FileGuid.ToString();
//int contractRefNo = Convert.ToInt32(asphdnFileMasterld.Value.ToString());
//Session(WebConstants.CONTRACT_REF_NO_SESSION_KEY] = contractRefNo;
asptxtReasonForRejection.Text = string.Empty;/**
* Always returns true.
*/
public boolean isAvailable()
{
return false;
}// format matched kk:mm:ss EEE, MMM dd, yyyy
Pattern timeMatcher = Pattern.Compile("\\d*:\\d*:\\d* \\w*, \\w* \\d*, \\d*"); //TODO: ...
//FIXME: ...
//HACK, NOTE, WARNING"Don’t comment bad code — rewrite it!"
B. Kernighan, P. Plauger "The Elements of Programming Style"
Плохо
customer.CalculateCredit ( fromDate );
customer.CalculateCredit(fromDate , toDate);
if ( IsValid ) i ++Хорошо
if(customer.IsValid && customer.Credit == 0.0)
position = new Location(position.x + 10, position.y);
return IsValid ? cdd : DateTime.MaxDate;using MVCS.Diff4Big.Domain.ImageEntities;
using MVCS.Diff4Big.Domain.Specifications;
namespace MVCS.Diff4Big.Domain.Comparison.FT {
public class ByteByByte : ITileComparator {
private readonly LengthBased lengthBased = new LengthBased();
public IDeltaContainer Compare(IContentTile tile1, IContentTile tile2, ChangeType changeType) {
var bytesRangeEqualsSpecification = new BytesRangeEquals(tile1.Content);
var baseResult = lengthBased.Compare(tile1, tile2, changeType);
if (baseResult != null) return baseResult;
if (bytesRangeEqualsSpecification.IsSatisfiedBy(tile2.Content)) return null;
if (changeType == ChangeType.ImageThematicalTile) {
return new ThematicalTileContainer((ThematicalBig1Tile) tile2.Clone());}
return new TileContainer((IContentTile) tile2.Clone(), changeType);
}
}
}using MVCS.Diff4Big.Domain.ImageEntities;
using MVCS.Diff4Big.Domain.Specifications;
namespace MVCS.Diff4Big.Domain.Comparison.FT {
public class ByteByByte : ITileComparator {
private readonly LengthBased lengthBased = new LengthBased();
public IDeltaContainer Compare(IContentTile tile1, IContentTile tile2, ChangeType changeType) {
var bytesRangeEqualsSpecification = new BytesRangeEquals(tile1.Content);
var baseResult = lengthBased.Compare(tile1, tile2, changeType);
if (baseResult != null) return baseResult;
if (bytesRangeEqualsSpecification.IsSatisfiedBy(tile2.Content)) return null;
if (changeType == ChangeType.ImageThematicalTile) {
return new ThematicalTileContainer((ThematicalBig1Tile) tile2.Clone());
}
return new TileContainer((IContentTile) tile2.Clone(), changeType);
}
}
} if (a == 0);
a++;
while(a++ != magic_number);
a = a << 2; if (a == 0) {
a++;
}
while(a++ != magic_number) {}
a = a << 2;Всегда оставляй лагерь чище, чем ты его нашел.
Вопросы?